Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use OpenAPI definitions as configuration schema #1022

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

Aareksio
Copy link
Member

@Aareksio Aareksio commented Jun 27, 2020

Description

As title says, the use of GET /Type and GET /Structure was replaced by parsing OpenAPI documentation available in /swagger/ASF/swagger.json. I focused on making existing components work with the mew definition and not much else, to reduce scope of the PR.

Please test it with latest ASF release (V4.2.3.4), as it relies on introduced changes.

Additional information

Closes #871

The code style may be not consistent with the project, our eslint settings are very scuffed (eg. existing code doesn't follow latest rules from plugins) and fixing the code style is out of the scope for this PR.

TODO

  • Default model for new bot - currently we use empty object as a model, which breaks the inputs and sets wrong defaults on fields which didn't break.
  • Extensive testing - last thing we want is to break existing configs, and this change has full potential to do so
  • Adjust UI settings to follow new schema definitions
  • Test if the update didn't break handling of sensitive properties (Bot Config)

Follow up

  • Cleanup input logic, normalize variable names
  • Abstract schema preparation - currently almost the same logic is repeated in at least 3 components
  • Cleanup unused code left from parsing old definitions

Checklist

  • My pull request is not a duplicate
  • I added a descriptive title to this pull request
  • My code follows the code style of this project
  • I have performed a self-review of my own code

@@ -76,26 +77,24 @@
},
methods: {
componentFromField(field) {
if (field.format === 'flags') return InputFlag;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good idea to test if it's enum first, then if it's enum flags. I can't guarantee that swagger won't decide to mark some other type with flags format just because.

label,
flag
}))
.filter(({ flag }) => flag % 2 === 0 || flag === 1);
Copy link
Member

@JustArchi JustArchi Jun 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I understand this code right but example value of 6 is not a standalone flag, it's 4 + 2.

Standalone flag is a value which is equal to 2^n, where n is in range from 0 to 64 (but in reality we don't really have that many, up to 16 or so, so you can stick with max safe value from js).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch, of course I didn't think too much coding this line, it was supposed to check whether the value is single flag (2^n), gonna fix ;)

Base automatically changed from master to main January 20, 2021 20:02
@MrBurrBurr MrBurrBurr marked this pull request as ready for review April 3, 2021 06:23
@MrBurrBurr MrBurrBurr marked this pull request as draft April 3, 2021 06:23
@MrBurrBurr
Copy link
Member

Happy belated birthday to our first 1-year old PR 🥳

@Abrynos
Copy link
Member

Abrynos commented Jul 22, 2021

I'd like to mention ASFs OpenAPI extensions here.

Since there is no official documentation of them on ASFs wiki, I'll just link the monologue of @JustArchi from the discord server for posterity: https://ptb.discord.com/channels/267292556709068800/332735075315744768/859854787634004049

@Aareksio
Copy link
Member Author

Aareksio commented Jul 4, 2022

Happy 2nd anniversary. Maybe one day.

@MrBurrBurr MrBurrBurr marked this pull request as ready for review July 23, 2022 00:05
@MrBurrBurr MrBurrBurr marked this pull request as draft July 23, 2022 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing text in bot config editor
4 participants